fix(pods): Fix invalid react-native/../react-native resolution for aliases#4232
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Updates React Native package.json discovery logic in the gesture handler CocoaPods/Ruby utilities to resolve the React Native directory more directly and adjust fallback behavior.
Changes:
- Change
try_to_parse_react_native_package_jsonto accept a React Native directory (instead of node_modules root) and readpackage.jsonfrom that directory. - Update
get_react_native_minor_versionto derive the React Native directory viarequire.resolve('react-native/package.json')and adjust the ENV fallback path.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Hi @kitten! Is this problem specific to monorepo? Today I used newly created react-native-tvos to investigate our buttons and I didn't encounter any problems with doing
Unfortunately, after this change |
|
I've renamed them. I assume we'd have to then do something similar in the other script in the related PRs, but the naming there is suspiciously generic. I'll leave it up to the review there though
No, not necessarily. For the TvOS case, as mentioned, it's triggerable in isolated installations, since we try to re-enter the react-native folder that's not named 'react-native' Generically, this applies to any case where we resolve through a symlink where the folder of the resolved package for react-native isn't called react-native, which can likely be triggered in other ways, like with |
…aliases (software-mansion#9592) ## Description Related: software-mansion/react-native-gesture-handler#4232 (same fix for `react-native-gesture-handler`) The pod scripts attempt to resolve `react-native/package.json` to get to the `version` and perform a version check. It however performs a redundant `react-native/../react-native` resolution i.e. it essentially performs `path.dirname(require.resolve('react-native/package.json'))` (correct), but then joins this with `${dir}/..`, then re-joins this with `react-native`. Re-entering the `react-native` directory can be invalid in case of symlinks in the `node_modules` structure. The easiest way to reproduce an issue is to use react-native-tvos's pattern of aliasing `"react-native": "npm:react-native-tvos@x.x.x"`. With isolated dependencies this will point a symlink at a directory that's named `react-native-tvos` and not `react-native`. The also applies to local `link:` dependency specifiers. Essentially, the directory name `react-native` is unnecessarily enforced. This would lead to a cryptic `file: no implicit conversion of nil into String.` error ## Test plan - Existing `pod install` in working projects should continue passing unchanged - https://github.com/kitten/douglowder--with-monorepo-tv-test/tree/init-pnpm - `pnpm i` then in `apps/tv` run `pnpm prebuild` (which runs `pod install`) --------- Co-authored-by: Tomasz Żelawski <tzelawski@gmail.com>
## Description In ruby, functions and variables are global by design. In #4232 we dealt with it by changing name of exported function. However, the correct approach would be to keep our utils in separate module. This has also been introduced in [Reanimated](software-mansion/react-native-reanimated#9617). ## Test plan `pod install`
## Description Cherry pick thread for release 2.32 ## List of PRs | PR | Original commit | Picked commit | Without confilct | |:----------:|:-------------:|:------:|:--------:| | [[Android] Remove `getChildInDrawingOrderAtIndex`](#4156) | [f1f09bc](f1f09bc) | [d76f2d9](d76f2d9) | ❌ | | [[iOS] Fix duplicated `testID`](#4186) | [7feab6e](7feab6e) | [d1173ec](d1173ec) | ❌ | | [[Android] Fix discrete gestures not triggering `onFinalize`](#4196) | [117087e](117087e) | [06212cf](06212cf) | ✅ | | [[Android] Fix wrong reported number of pointers](#4218) | [f17bcbd](f17bcbd) | [5fd30f6](5fd30f6) | ❌ | | [fix(pods): Fix invalid `react-native/../react-native` resolution for aliases](#4232) | [9a9f8b4](9a9f8b4) | [cacf6c0](cacf6c0) | ✅ | | [Move ruby utils to module](#4245) | [62d0d52](62d0d52) | [3884f30](3884f30) | ❌ | | [[iOS] Fix handler retrieval and mismatched coordinate space](#4199) | [b1da40b](b1da40b) | [2b48a8e](2b48a8e) | ❌ | | [[iOS] Don't iterate to root while resolving recognizer](#4202) | [fba4dcc](fba4dcc) | [b81f2aa](b81f2aa) | ❌ | | [Support React Native 0.86](#4166) | [96dfc00](96dfc00) | [7df6d1a](7df6d1a) | ❌ | | [Fix `RNRenderer` import for React Native 0.86](#4160) | [0417378](0417378) | [6775d33](6775d33) | ❌ | | [Bump `expo` to stable 56](#4227) | [45e6ac8](45e6ac8) | [0478bb5](0478bb5) | ❌ | | [[macOS] Bump example app](#3993) | [8ec0820](8ec0820) | [cfc5dd4](cfc5dd4) | ❌ | | [Bump Reanimated](#4040) | [23d96d9](23d96d9) | [a2101eb](a2101eb) | ❌ | | [fix(Android): apply `numberOfPointers` config to the pointer requirement in `LongPressGestureHandler`](#4253) | [a70c6e3](a70c6e3) | [af40f9b](af40f9b) | ✅ | ## Test plan Tested that example apps are built correctly --------- Co-authored-by: Pieter De Baets <pieter.debaets@gmail.com> Co-authored-by: Mathieu Acthernoene <zoontek@gmail.com> Co-authored-by: Jakub Piasecki <jakub.piasecki@swmansion.com> Co-authored-by: Phil Pluckthun <phil@kitten.sh> Co-authored-by: prashanFOMO <prashan@fomo.family> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Description
Related: software-mansion/react-native-reanimated#9592 (same fix for
react-native-reanimatedandreact-native-worklets)The pod script attempts to resolve
react-native/package.jsonto get to theversionand perform a version check. It however performs a redundantreact-native/../react-nativeresolution i.e. it essentially performspath.dirname(require.resolve('react-native/package.json'))(correct), but then joins this with${dir}/.., then re-joins this withreact-native.Re-entering the
react-nativedirectory can be invalid in case of symlinks in thenode_modulesstructure. The easiest way to reproduce an issue is to use react-native-tvos's pattern of aliasing"react-native": "npm:react-native-tvos@x.x.x". With isolated dependencies this will point a symlink at a directory that's namedreact-native-tvosand notreact-native. The also applies to locallink:dependency specifiers. Essentially, the directory namereact-nativeis unnecessarily enforced.This would lead to a cryptic
file: no implicit conversion of nil into String.errorTest plan
pod installin working projects should continue passing unchangedpnpm ithen inapps/tvrunpnpm prebuild(which runspod install)